[Backport 7.79.x] fix(appsec/nginx): reject cross-namespace --configmap refs in pod mutation - #51710
[Backport 7.79.x] fix(appsec/nginx): reject cross-namespace --configmap refs in pod mutation#51710dd-octo-sts[bot] wants to merge 1 commit into
Conversation
…ation (#51635) ### What does this PR do? Fixes a confused-deputy vulnerability in the Cluster Agent's AppSec ingress-nginx admission mutator. The webhook previously extracted the namespace from the pod's `--configmap=<ns>/<name>` argument and used it verbatim for ConfigMap `Get`/`Create`/`Update` calls. Combined with the DCA's cluster-wide `configmaps` permissions, a low-privileged tenant with `create pods` rights in one namespace could trigger writes to ConfigMaps in arbitrary namespaces. Changes: - **`pkg/clusteragent/appsec/nginx/sidecar.go`** — `findControllerConfigMapArg` now requires the `<ns>` portion to match the pod's own namespace (resolving `$(POD_NAMESPACE)` first) and rejects empty names. On rejection, `MutatePod` returns `(false, nil)` to preserve fail-open admission semantics — the pod is admitted unmodified. - **`pkg/clusteragent/appsec/nginx/events.go`** — New `CrossNamespaceConfigMapRefused` warning event on the **pod** (not the IngressClass) so the diagnostic lands in the tenant's namespace where their operator can see it. - **`pkg/clusteragent/appsec/nginx/configmap.go`** — Defense-in-depth `validateConfigMapTarget` (DNS-1123 validation) at the entry of `createOrUpdateDDConfigMap`, covering both webhook and reconciler paths. - **Tests** — `TestMutatePod_CrossNamespaceConfigMapRefused` (the bisect anchor) asserts no API calls escape and the pod spec is unmodified. `TestFindControllerConfigMapArg` extended from 3 cases to 10 covering same-ns, foreign ns, `kube-system` reference, leading/trailing slash, multi-container priority. - **Release note** — `releasenotes/notes/fix-appsec-nginx-configmap-confused-deputy-*.yaml` (security section). The introducing change was PR #49318 (Agent 7.78.0). All releases ≥7.78.0 are affected; backports to `7.78.x`, `7.79.x`, `7.80.x` will follow. ### Motivation Tracking: [APPSEC-68212](https://datadoghq.atlassian.net/browse/APPSEC-68212). Internal vulnerability report `clusteragent-appsec-nginx-configmap-confused-deputy` (severity High, threat model k8s-tenant). Full mitigation plan: `.sisyphus/plans/clusteragent-appsec-nginx-configmap-confused-deputy-mitigation.md`. Pre-condition for exploitation: DCA with `cluster_agent.appsec.injector.enabled = true` (helm: `datadog.appsec.injector.enabled: true`), at least one ingress-nginx `IngressClass` (`controller: k8s.io/ingress-nginx`), and a tenant with `create pods` permission in any namespace. ### Describe how you validated your changes **1. Automated tests (run in CI):** ```bash dda inv test --targets=./pkg/clusteragent/appsec/nginx # 57/57 passed dda inv test --targets=./pkg/clusteragent/admission/mutate/appsec # 35/35 passed dda inv linter.go --targets=./pkg/clusteragent/appsec/nginx # 0 issues dda inv linter.releasenote # passed ``` `TestMutatePod_CrossNamespaceConfigMapRefused` is the **bisect anchor**: it fails against the unpatched code (which accepted `--configmap=kube-system/coredns` verbatim) and passes against this patch. **2. Live exploit reproduction (k3s, rancher-desktop, 7.78.0 + this patch):** Setup the DCA with my patched binary: ```bash # Overlay the patched binary on the 7.78.0 base image cat > Dockerfile.overlay <<'DOCKERFILE' FROM datadog/cluster-agent:7.78.0 COPY bin/datadog-cluster-agent/datadog-cluster-agent /opt/datadog-agent/bin/datadog-cluster-agent DOCKERFILE docker build --platform linux/arm64 -t datadog/cluster-agent:7.78.0-fix -f Dockerfile.overlay . # Install DCA with AppSec ingress-nginx enabled cat > values.yaml <<'YAML' datadog: apiKey: "0000000000000000000000000000000000000000" appKey: "0000000000000000000000000000000000000000" clusterName: confused-deputy-test appsec: injector: enabled: true autoDetect: false proxies: [ingress-nginx] clusterAgent: image: repository: datadog/cluster-agent tag: 7.78.0-fix pullPolicy: IfNotPresent admissionController: enabled: true agents: { enabled: false } clusterChecksRunner: { enabled: false } YAML helm install dd datadog/datadog -n datadog --create-namespace -f values.yaml # Pre-condition: at least one ingress-nginx IngressClass kubectl apply -f - <<'YAML' apiVersion: networking.k8s.io/v1 kind: IngressClass metadata: { name: nginx-test } spec: { controller: k8s.io/ingress-nginx } YAML ``` Apply the exploit pod from a low-privileged tenant namespace: ```yaml # attacker-pod.yaml apiVersion: v1 kind: Namespace metadata: { name: attacker-ns } --- apiVersion: v1 kind: Pod metadata: name: confused-deputy-poc namespace: attacker-ns labels: app.kubernetes.io/name: ingress-nginx app.kubernetes.io/component: controller spec: containers: - name: c image: registry.k8s.io/ingress-nginx/controller:v1.15.1 args: - /nginx-ingress-controller - --configmap=kube-system/coredns - --election-id=test ``` **Expected (and observed) outcomes:** | Assertion | Command | Result | |---|---|---| | Pod admitted (fail-open) | `kubectl get pod -n attacker-ns confused-deputy-poc` | ✅ admitted, no admission error | | Args UNMODIFIED | `kubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.containers[0].args}'` | ✅ `--configmap=kube-system/coredns` preserved | | No init container injected | `kubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.initContainers}'` | ✅ empty | | No DD ConfigMap in `kube-system` | `kubectl get cm -n kube-system \| grep -i datadog-appsec` | ✅ none | | Warning event on pod | `kubectl get events -n attacker-ns --field-selector involvedObject.name=confused-deputy-poc` | ✅ `Warning CrossNamespaceConfigMapRefused AppSec nginx mutation skipped: --configmap references a namespace different from the pod's namespace; refusing to mutate to avoid confused-deputy ConfigMap writes: pod attacker-ns/confused-deputy-poc, arg "--configmap=kube-system/coredns"` | | DCA log line | `kubectl logs -n datadog deploy/dd-datadog-cluster-agent \| grep "AppSec mutation skipped"` | ✅ `WARN \| nginx AppSec mutation skipped for pod attacker-ns/confused-deputy-poc: --configmap references a namespace different from the pod's namespace` | **3. Regression check — legitimate ingress-nginx deployments still work** Pods using the upstream Helm default (`--configmap=$(POD_NAMESPACE)/ingress-nginx-controller`) and pods using a literal same-namespace ref (`--configmap=ingress-nginx/my-config` when the pod is in `ingress-nginx`) are accepted and mutated normally. Covered by `TestFindControllerConfigMapArg/standard_$(POD_NAMESPACE)_form_is_accepted` and `.../hardcoded_same_namespace_is_accepted`. ### Additional Notes - **`qa/rc-required` is required** — admission webhook changes touch cross-component behavior (DCA ↔ kube-apiserver ↔ node agents) per `AGENTS.md` guidance. - The fix is **fail-open**: rejection results in the pod being admitted unmodified with a warning event and log line — never a failed admission. Legitimate ingress-nginx deployments using `$(POD_NAMESPACE)/...` (the upstream Helm default) are unaffected. - `createOrUpdateDDConfigMap` gains DNS-1123 validation as defense-in-depth. It is a no-op for the reconciler path (whose namespace/name come from a label-filtered informer watch and are already valid Kubernetes objects) and catches any future code path that bypasses `findControllerConfigMapArg`. - Follow-ups deferred to separate Jira tickets per the plan: - E2E test in `test/new-e2e/tests/clusteragent/appsec/` (§4.5) - Owner-reference pre-check on ingress-nginx pods (§5.2) - `ValidatingAdmissionPolicy` for ConfigMap creation scope (§6 Option C) Co-authored-by: eliott.bouhana <eliott.bouhana@datadoghq.com> (cherry picked from commit 9ae4dea) ___ Co-authored-by: Eliott B <47679741+eliottness@users.noreply.github.com>
|
Files inventory check summaryFile checks results against ancestor 98c7c0e4: Results for datadog-agent_7.79.2.git.2.ab93bd2.pipeline.116706147-1_amd64.deb:No change detected |
|
This pull request has been automatically marked as stale because it has not had activity in the past 15 days. It will be closed in 30 days if no further activity occurs. If this pull request is still relevant, adding a comment or pushing new commits will keep it open. Also, you can always reopen the pull request if you missed the window. Thank you for your contributions! |
|
This pull request was automatically closed because it has been stale for 15 days with no activity. If this pull request is still relevant, please reopen it or create a new pull request with updated information. Thanks! |
Backport 9ae4dea from #51635.
What does this PR do?
Fixes a confused-deputy vulnerability in the Cluster Agent's AppSec ingress-nginx admission mutator. The webhook previously extracted the namespace from the pod's
--configmap=<ns>/<name>argument and used it verbatim for ConfigMapGet/Create/Updatecalls. Combined with the DCA's cluster-wideconfigmapspermissions, a low-privileged tenant withcreate podsrights in one namespace could trigger writes to ConfigMaps in arbitrary namespaces.Changes:
pkg/clusteragent/appsec/nginx/sidecar.go—findControllerConfigMapArgnow requires the<ns>portion to match the pod's own namespace (resolving$(POD_NAMESPACE)first) and rejects empty names. On rejection,MutatePodreturns(false, nil)to preserve fail-open admission semantics — the pod is admitted unmodified.pkg/clusteragent/appsec/nginx/events.go— NewCrossNamespaceConfigMapRefusedwarning event on the pod (not the IngressClass) so the diagnostic lands in the tenant's namespace where their operator can see it.pkg/clusteragent/appsec/nginx/configmap.go— Defense-in-depthvalidateConfigMapTarget(DNS-1123 validation) at the entry ofcreateOrUpdateDDConfigMap, covering both webhook and reconciler paths.TestMutatePod_CrossNamespaceConfigMapRefused(the bisect anchor) asserts no API calls escape and the pod spec is unmodified.TestFindControllerConfigMapArgextended from 3 cases to 10 covering same-ns, foreign ns,kube-systemreference, leading/trailing slash, multi-container priority.releasenotes/notes/fix-appsec-nginx-configmap-confused-deputy-*.yaml(security section).The introducing change was PR #49318 (Agent 7.78.0). All releases ≥7.78.0 are affected; backports to
7.78.x,7.79.x,7.80.xwill follow.Motivation
Tracking: APPSEC-68212. Internal vulnerability report
clusteragent-appsec-nginx-configmap-confused-deputy(severity High, threat model k8s-tenant). Full mitigation plan:.sisyphus/plans/clusteragent-appsec-nginx-configmap-confused-deputy-mitigation.md.Pre-condition for exploitation: DCA with
cluster_agent.appsec.injector.enabled = true(helm:datadog.appsec.injector.enabled: true), at least one ingress-nginxIngressClass(controller: k8s.io/ingress-nginx), and a tenant withcreate podspermission in any namespace.Describe how you validated your changes
1. Automated tests (run in CI):
TestMutatePod_CrossNamespaceConfigMapRefusedis the bisect anchor: it fails against the unpatched code (which accepted--configmap=kube-system/corednsverbatim) and passes against this patch.2. Live exploit reproduction (k3s, rancher-desktop, 7.78.0 + this patch):
Setup the DCA with my patched binary:
Apply the exploit pod from a low-privileged tenant namespace:
Expected (and observed) outcomes:
kubectl get pod -n attacker-ns confused-deputy-pockubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.containers[0].args}'--configmap=kube-system/corednspreservedkubectl get pod -n attacker-ns confused-deputy-poc -o jsonpath='{.spec.initContainers}'kube-systemkubectl get cm -n kube-system | grep -i datadog-appseckubectl get events -n attacker-ns --field-selector involvedObject.name=confused-deputy-pocWarning CrossNamespaceConfigMapRefused AppSec nginx mutation skipped: --configmap references a namespace different from the pod's namespace; refusing to mutate to avoid confused-deputy ConfigMap writes: pod attacker-ns/confused-deputy-poc, arg "--configmap=kube-system/coredns"kubectl logs -n datadog deploy/dd-datadog-cluster-agent | grep "AppSec mutation skipped"WARN | nginx AppSec mutation skipped for pod attacker-ns/confused-deputy-poc: --configmap references a namespace different from the pod's namespace3. Regression check — legitimate ingress-nginx deployments still work
Pods using the upstream Helm default (
--configmap=$(POD_NAMESPACE)/ingress-nginx-controller) and pods using a literal same-namespace ref (--configmap=ingress-nginx/my-configwhen the pod is iningress-nginx) are accepted and mutated normally. Covered byTestFindControllerConfigMapArg/standard_$(POD_NAMESPACE)_form_is_acceptedand.../hardcoded_same_namespace_is_accepted.Additional Notes
qa/rc-requiredis required — admission webhook changes touch cross-component behavior (DCA ↔ kube-apiserver ↔ node agents) perAGENTS.mdguidance.$(POD_NAMESPACE)/...(the upstream Helm default) are unaffected.createOrUpdateDDConfigMapgains DNS-1123 validation as defense-in-depth. It is a no-op for the reconciler path (whose namespace/name come from a label-filtered informer watch and are already valid Kubernetes objects) and catches any future code path that bypassesfindControllerConfigMapArg.test/new-e2e/tests/clusteragent/appsec/(§4.5)ValidatingAdmissionPolicyfor ConfigMap creation scope (§6 Option C)